fix: check trimmed text for heading marker so note bodies reach embeddings - #14
Open
arimu1 wants to merge 1 commit into
Open
fix: check trimmed text for heading marker so note bodies reach embeddings#14arimu1 wants to merge 1 commit into
arimu1 wants to merge 1 commit into
Conversation
…dings
gray-matter leaves a leading newline after stripping YAML frontmatter,
so a naive split('\n\n') on note content yields '\n# Title' as the
first paragraph. Both firstParagraph() in store.ts (search excerpts)
and buildEmbeddingText() in embedder.ts checked p.startsWith('#')
without trimming first, so that leading-newline heading was never
recognized as a heading — it was treated as the "first paragraph" and
used verbatim. In embedder.ts this meant note bodies never entered the
embedding vector at all (embedding text was title + tags + title),
collapsing semantic search recall to title matches only.
Extracted a shared firstBodyParagraph() helper (src/lib/text.ts) that
trims each candidate paragraph before checking for '#', and used it at
both call sites.
Added unit tests for the helper directly, plus regression tests for
store.searchVector() excerpts and Embedder.buildEmbeddingText()
against gray-matter-style leading-newline content.
Fixes obra#6.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Semantic search (
kg_search) effectively ignores note bodies. Excerpts show the# Titleheading instead of body text, and any note whose title doesn't literally contain the query keyword is unreachable via semantic search — even when the body is a strong match.Root cause
gray-matterleaves a leading newline after stripping the YAML frontmatter, socontent.split(/\n\n+/)[0]is typically'\n# Title', not'# Title'. Two call sites checkedp.startsWith('#')before trimming, so that leading-newline heading was never recognized as a heading:src/lib/store.ts:283(firstParagraph, used forsearchVectorexcerpts) — the heading slips through as the "first paragraph" shown in results.src/lib/embedder.ts:37(buildEmbeddingText) — higher impact:split[0]was taken unconditionally with no heading check at all, so the embedding text ends up astitle + tags + "# title". The note body never enters the embedding vector, which collapses semantic recall to title matches only.Verified this against the current code on
main(commit1d2481e) before making any changes.Fix
Extracted a shared
firstBodyParagraph()helper (src/lib/text.ts) that trims each candidate paragraph before checking for a leading#, per the fix suggested in the issue. Used it at both call sites:store.ts'sfirstParagraph()now delegates to the helper before length-capping.embedder.ts'sbuildEmbeddingText()now uses the helper (and trims the result) instead of an unconditionalsplit[0].Testing
test/text.test.ts: direct unit tests forfirstBodyParagraph, including the gray-matter leading-newline case.test/embedder.test.tsassertingbuildEmbeddingTextincludes the body paragraph (not the# Titleheading) when content has a leading newline before the heading.test/store.test.tsassertingsearchVectorexcerpts contain the body paragraph, not the heading, for the same leading-newline shape.npx vitest run→ 13 files, 92 tests, all passing.npm run buildbefore and after the change and diffed the output: identical 32 pre-existingtscerrors ingraph.ts/mcp/index.ts/embedder.ts(unrelated dependency-version type issues already present onmain), confirming this change introduces no new type errors.As noted in the issue, existing indexes will need
kg index --forceafter upgrading, since embedding content changes.Fixes #6.
This PR was developed with AI assistance (Claude Code, model Claude Fable 5) with human review.